Skip to content

Add some basic documentation to Render, Pure and Hook #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 26, 2020

Conversation

ford-prefect
Copy link
Contributor

This might help newcomers grok the types a bit more easily. I'm sure
this can be improved on, but might be okay as a start.

@ford-prefect
Copy link
Contributor Author

(also not 100% sure I've wrapped my head around this correctly, so please let me know if I got it wrong and I can fix it)

@maddie927
Copy link
Member

Nice, thank you!

I think there's a missing space on line 133.

Pure isn't quite right, but I'm not sure how to explain it succinctly without assuming some understanding of the pure function and indexed monads, at which point it mostly speaks for itself.. maybe it'd be better to just remove it, as it isn't used much and can almost always be replaced by adding pure to any non-Render function. Maybe something like "Type alias for Render x x _, used to lift otherwise pure functionality into the Render type. Not commonly used." (I'm abbreviating a little still)

Hook is just a shorthand for Render x (newHook x) _ so it's easier to recognize a function as a hook. I almost called Render itself Hook, but this seemed to leave it a little bit more flexible for future use.

Sorry, that's not very constructive feedback, just a small thought dump. What I really need to get around to is a blog post which introduces concepts alongside relevant code, with a bit more room to explain.

This might help newcomers grok the types a bit more easily. I'm sure
this can be improved on, but might be okay as a start.
@ford-prefect
Copy link
Contributor Author

Thank you for the explanation!

I've modified the PR to more closely reflect how you described things. If it still feels iffy, I could maybe drop the annotations on Pure and Hook?

Having a blog post about the library whenever you are able would certainly be awesome. :)

Copy link
Member

@maddie927 maddie927 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, ty!

@maddie927 maddie927 merged commit 9175d48 into purescript-react:master Feb 26, 2020
@ford-prefect ford-prefect deleted the doc branch February 26, 2020 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants